feat: treat request update as patch input#285
Conversation
|
Hi Pookie :D |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #285 +/- ##
========================================
+ Coverage 1.88% 9.20% +7.32%
========================================
Files 86 135 +49
Lines 3289 5626 +2337
Branches 24 24
========================================
+ Hits 62 518 +456
- Misses 3227 5092 +1865
- Partials 0 16 +16
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
eric-kitagawa
changed the title
eric-kitagawa
added
feature
| utils.ApplyPtr(&r.CompletedAt, patch.CompletedAt) | ||
| utils.ApplyPtr(&r.Notes, patch.Notes) | ||
| } | ||
|
|
There was a problem hiding this comment.
Thoughts on leaving this in the model? It didn't feel right to place in the handler or repo layer. My thought was since this is just a mapping function on the exact same fields as the model itself it should sit side by side with it.
Dao-Ho
left a comment
Dao-Ho
left a comment
There was a problem hiding this comment.
one cmt on explicitness. FIRE SHIP 🚀
|
|
||
| // RequestPatchInput is the body for PUT /request/:id — all fields are optional. | ||
| // Only non-nil fields are applied; the rest are copied from the current version. | ||
| type RequestPatchInput struct { |
There was a problem hiding this comment.
Patch and Update is used synonymously in your endpoint, I'd stick to one here.
I prefer update but up to your preference
| if src != nil { | ||
| *dst = src | ||
| } | ||
| } |
There was a problem hiding this comment.
I think this is ok for now but this feels like a SQL COALESCE() that could be done at the SQL level
Definitely think vars and parameters could be named better for explicitness.
| utils.ApplyPtr(&r.CompletedAt, patch.CompletedAt) | ||
| utils.ApplyPtr(&r.Notes, patch.Notes) | ||
| } | ||
|
|
3 participants
Refactored the update request endpoint to support accepting partial inputs